Ensure TaskInstance.end_date and duration are populated before invoking failure callbacks#52729
Conversation
- Set ti.end_date before creating TaskState in exception handlers - Ensures on_failure_callback context has proper end_date and duration - Fixes race condition where callbacks received incomplete TaskInstance data - Add test to verify callback context contains required timing information
387adee to
896d0c1
Compare
|
Can you check/test that success callbacks also have an end date, and then we can get this merged |
amoghrajesh
left a comment
There was a problem hiding this comment.
It would be nice to test success call backs too. Looks good otherwise
|
hi @ashb @amoghrajesh, i tested both success and failure callbacks to make sure that they have the end date and duration info, the following are the log results: $ python -m pytest task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks -v
============================= test session starts ==============================
collecting ... collected 11 items
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_calls_callback[success] PASSED [ 9%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_calls_callback[skipped] PASSED [ 18%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_calls_callback[failure] PASSED [ 27%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_calls_callback[retry] PASSED [ 36%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_on_failure_callback_context PASSED [ 45%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_on_success_callback_context PASSED [ 54%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_both_callbacks_have_timing_info PASSED [ 63%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_not_fail_on_failed_callback[success] PASSED [ 72%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_not_fail_on_failed_callback[skipped] PASSED [ 81%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_not_fail_on_failed_callback[failure] PASSED [ 90%]
task-sdk/tests/task_sdk/execution_time/test_task_runner.py::TestTaskRunnerCallsCallbacks::test_task_runner_not_fail_on_failed_callback[retry] PASSED [100%]
========================= 11 passed, 1 warning in 1.93s =========================I had to update the |
amoghrajesh
left a comment
There was a problem hiding this comment.
LGTM +1 because it has been tested.
@ashb wdyt?
|
Unrelated failure, retriggering it |
|
Awesome work, congrats on your first merged pull request! You are invited to check our Issue Tracker for additional contributions. |
…efore invoking failure callbacks (#52729) * Fix: Set TaskInstance end_date before failure callbacks (#52630) - Set ti.end_date before creating TaskState in exception handlers - Ensures on_failure_callback context has proper end_date and duration - Fixes race condition where callbacks received incomplete TaskInstance data - Add test to verify callback context contains required timing information * Enhance task callbacks to include end_date and duration metrics (cherry picked from commit eda89ae) Co-authored-by: Ranuga Disansa <79456372+Programmer-RD-AI@users.noreply.github.com>
…efore invoking failure callbacks (#52729) (#54458) * Fix: Set TaskInstance end_date before failure callbacks (#52630) - Set ti.end_date before creating TaskState in exception handlers - Ensures on_failure_callback context has proper end_date and duration - Fixes race condition where callbacks received incomplete TaskInstance data - Add test to verify callback context contains required timing information * Enhance task callbacks to include end_date and duration metrics (cherry picked from commit eda89ae) Co-authored-by: Ranuga Disansa <79456372+Programmer-RD-AI@users.noreply.github.com>
This change addresses a race condition in the task runner where downstream failure callbacks might observe an uninitialized or stale
end_dateanddurationon theTaskInstance. By explicitly settingti.end_datebefore constructing theTaskStatein all exception handlers, we guarantee that anyon_failure_callbackreceives a fully populated context, preventing confusing debug sessions and user‐defined callback failures.Key Changes:
ti.end_dateearly in bothAirflowFailExceptionand generic exception handlers, before creating theTaskState.durationautomatically by virtue of the populatedend_date, ensuring callbacks can compute and report accurate runtime metrics.test_task_runner_on_failure_callback_context) that verifies bothend_dateand non‐negativedurationare present in the failure callback context.TaskInstancedata, leading to misleading or missing timing information.This PR closes #52630